Skip to content

Lazy discovery on demand#215

Merged
rytilahti merged 4 commits intorytilahti:masterfrom
syssi:feature/discover-on-request
Feb 13, 2018
Merged

Lazy discovery on demand#215
rytilahti merged 4 commits intorytilahti:masterfrom
syssi:feature/discover-on-request

Conversation

@syssi
Copy link
Copy Markdown
Collaborator

@syssi syssi commented Feb 12, 2018

I want to reduce the number of discovery messages of specific devices (Philips Lights).

@yawor
Copy link
Copy Markdown
Contributor

yawor commented Feb 12, 2018

@syssi in the async code I've started working on (not much time recently to move it forward) I'm running discovery independently from the requests. It runs on timer every minute or so. There's also an async lock which keeps the command from running if library is waiting for discovery response.
I can push my initial async code to a PR but there's not much there yet.

@syssi
Copy link
Copy Markdown
Collaborator Author

syssi commented Feb 12, 2018

@rytilahti The lazy device discovery works fine for philips lights without drawbacks so far. Are you happy with the order of the init parameters?

@rytilahti
Copy link
Copy Markdown
Owner

rytilahti commented Feb 12, 2018

That looks fine for me (the "force discovery" was merely a hack to make it work after some FW update), but let me run this for a day or so on all my devices just to be sure. I'm not sure about the parameter order, but I think the lazy discovery should be set as True per default. Those devices with problems can be forced to do it every time.

@yawor iirc some miio protocol implementation (maybe it was the javascript one) used to do updates every 180 or 200 seconds. I'm unsure if they had any data to back that up though. Having an initial PR for async sounds good though! In a couple of weeks homeassistant will drop python 3.4 support, which probably will lead the way for dropping it also from this library (if @syssi is not too much against it :-)).

@rytilahti rytilahti merged commit deb072c into rytilahti:master Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants